Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypt keys with trussed-auth #127

Closed
wants to merge 13 commits into from
Closed

Conversation

sosthene-nitrokey
Copy link
Collaborator

@sosthene-nitrokey sosthene-nitrokey commented Mar 20, 2023

Built on top of #125 and trussed-dev/trussed-auth#17

This PR adds encryption for the user keys that will end up stored on the external flash.

@sosthene-nitrokey
Copy link
Collaborator Author

This PR encrypts:

  • The 3 main keys
  • The AES keys
  • The private use data objects that are read-restricted. Because of the overhead caused by the nonce and tag in reduces the maximum size from 1024 to 980.

Am I missing something?

@sosthene-nitrokey
Copy link
Collaborator Author

The PIN length is still stored unencrypted but I don't see anyway to avoid this. Maybe store it in the internal flash?
The only really secure option I see would be to only store it in the volatile state and require a Verify call before CHANGE REFERENCE DATA, but that would violate the spec and it would also not work for the resetting code, which doesn't have a VERIFY call.

@jans23
Copy link
Member

jans23 commented Mar 20, 2023

Why do we need to store the PIN length?

@sosthene-nitrokey
Copy link
Collaborator Author

Because CHANGE REFERENCE DATA, the command to change the PIN, transmits the new and the old pin concatenated.
We have no way to split the input in the correct place if we don't know the current PIN length.

@jans23
Copy link
Member

jans23 commented Mar 20, 2023

Is this demanded by the OpenPGP Card specification?

@szszszsz
Copy link
Member

@jans23 Yes, that's the way of changing the PIN per specification - giving both concatenated without additional information.

@sosthene-nitrokey
Copy link
Collaborator Author

sosthene-nitrokey commented Mar 28, 2023

More detailed encryption mechanism:

User data that can be encrypted is. This applies to:

  • PGP keys for Signing, Decryption and authentication
  • The symmetric AES key
  • Private use data object 3

The same is true for admin data (private use data object 4).

All other data need to be accessible without verifying a PIN, and therefore cannot be encrypted using user-provided entropy.

Encryption uses keys wrapped using the user and admin PIN coming from trussed-auth.
Cryptographic keys are stored wrapped and are unwrapped only when necessary with the user PIN.

Encryption path

Encryption (normal) path:

  • user key: Obtained from trussed-auth's get_pin_key when the user calls Verify
  • admin key: Obtained from trussed-auth's get_pin_key when the admin calls Verify
  • resetting code key: Obtained from trussed-auth's get_pin_key when the resetting code calls ResetRetryCounter

The 3 PGP keys are stored wrapped with the user key. They are only stored unwrapped in volatile memory temporarly when being generated, imported or used (with some caching).

User key wrapping

This path is by itself incompatible with the OpenPGP standard.

Key generation

Only the admin pin is required generate and import PGP keys.
Without the user PIN, it would be impossible to wrap them.

User PIN unblock

The admin pin and the resetting code need to be able to reset the user pin, but the user must then still be able to unwrap the PGP keys.

User key "backups"

To adapt to the two above use cases, the user key is backed up twice.
One bakcup is wrapped with the admin key (ADMIN_USER_KEY_BACKUP) and the other is wrapped with the resetting code key (RC_USER_KEY_BACKUP).

Card administration

When the admin generates or imports a PGP key, the card uses the admin key to unwrap ADMIN_USER_KEY_BACKUP, giving it access to the user key.
The user key is then used to wrap the imported/generated PGP key.

User PIN reset

When RESET RETRY COUNTER is called, the card unwraps either ADMIN_USER_KEY_BACKUP or RC_USER_KEY_BACKUP depending on wether the admin key or the resetting code is used.
This gives the card access to the user key, which can unblock the user pin with trussed-auth's reset_pin_with_key.

Private use data objects

Objects are majoritarly readable without authentication and therefore are store in plain text.
There are two exceptions:

  • Private use data object 3 is also encrypted with the user key.
  • Private use data object 4 is encrypted with the admin key.

With a schema in nextcloud: https://cloud.nitrokey.com/f/522841

@sosthene-nitrokey sosthene-nitrokey force-pushed the trussed-auth-encryption branch 2 times, most recently from 68787ce to bd0dae6 Compare March 30, 2023 14:09
@sosthene-nitrokey sosthene-nitrokey force-pushed the trussed-auth-encryption branch 2 times, most recently from f618039 to c4d714f Compare March 30, 2023 15:06
Base automatically changed from trussed-auth to main April 4, 2023 15:46
@sosthene-nitrokey sosthene-nitrokey force-pushed the trussed-auth-encryption branch 2 times, most recently from 54c51fa to ebd5229 Compare April 7, 2023 08:52
@sosthene-nitrokey
Copy link
Collaborator Author

I'll break this PR into some smaller PRs for easier review.

This was introduced by rebasing on top of the rsa-backend migration
This should not be done since public keys are expected to be stored in persistent
storage since they can't be lazily derived from the private part that is encrypted
@sosthene-nitrokey
Copy link
Collaborator Author

PRs #134, #135, #136, #137, #138 are the parts of this PRs separated for easier review. They all point to the data-encryption branch so that they can be merged as soon as reviewed, and then we merge the data-encryption PR and close this one.

@robin-nitrokey
Copy link
Member

robin-nitrokey commented Apr 11, 2023

To do before the final merge:

@robin-nitrokey
Copy link
Member

Thanks for splitting up the PR! The code changes look good to me.

@sosthene-nitrokey
Copy link
Collaborator Author

Replaced by #139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Trussed-Auth for pin handling and encryption
4 participants